Skip to content

cmd/contour: add EnableXRateLimitHeaders config file setting#3457

Merged
skriss merged 1 commit intoprojectcontour:mainfrom
skriss:fix-3431
Mar 16, 2021
Merged

cmd/contour: add EnableXRateLimitHeaders config file setting#3457
skriss merged 1 commit intoprojectcontour:mainfrom
skriss:fix-3431

Conversation

@skriss
Copy link
Copy Markdown
Member

@skriss skriss commented Mar 8, 2021

Adds the EnableXRateLimitHeaders field to the config file's
RateLimitService block. When set to true, adds the X-RateLimit
headers to responses that required checking the RLS.

Closes #3431.

Signed-off-by: Steve Kriss krisss@vmware.com

@skriss skriss added this to the 1.14.0 milestone Mar 8, 2021
@skriss skriss requested a review from a team as a code owner March 8, 2021 18:38
@skriss skriss requested review from danehans and youngnick and removed request for a team March 8, 2021 18:38
@skriss
Copy link
Copy Markdown
Member Author

skriss commented Mar 8, 2021

I assumed that folks agreed with #3431 (comment), but I'd still appreciate input one way or another on that. Easy enough to change things if we want to switch to a non-boolean config option.

@skriss skriss requested a review from stevesloka March 8, 2021 18:39
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 8, 2021

Codecov Report

Merging #3457 (f6159f4) into main (7b18f01) will increase coverage by 0.05%.
The diff coverage is 64.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3457      +/-   ##
==========================================
+ Coverage   75.15%   75.21%   +0.05%     
==========================================
  Files          98       98              
  Lines        6586     6592       +6     
==========================================
+ Hits         4950     4958       +8     
+ Misses       1523     1521       -2     
  Partials      113      113              
Impacted Files Coverage Δ
cmd/contour/serve.go 0.00% <0.00%> (ø)
internal/envoy/v3/ratelimit.go 100.00% <100.00%> (ø)
internal/xdscache/v3/listener.go 93.36% <100.00%> (+0.03%) ⬆️
internal/k8s/log.go 69.56% <0.00%> (+6.52%) ⬆️

Copy link
Copy Markdown
Member

@sunjayBhatia sunjayBhatia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM assuming the boolean option is cool with everyone

@stevesloka stevesloka added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Mar 12, 2021
@skriss
Copy link
Copy Markdown
Member Author

skriss commented Mar 15, 2021

sounds like using the bool works for everyone, but I'll leave this open for one more day in case @youngnick or @xaleeks have further input.

Adds the EnableXRateLimitHeaders field to the config file's
RateLimitService block. When set to true, adds the X-RateLimit
headers to responses that required checking the RLS.

Closes projectcontour#3431.

Signed-off-by: Steve Kriss <krisss@vmware.com>
@skriss skriss merged commit 723081e into projectcontour:main Mar 16, 2021
@skriss skriss deleted the fix-3431 branch March 16, 2021 14:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-note Denotes a PR that will be considered when it comes time to generate release notes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

global rate limiting: allow configuring enable_x_ratelimit_headers

3 participants